-
Notifications
You must be signed in to change notification settings - Fork 519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Docs: Clarified Migration README: grammar, out-of-date, organization #2141
Docs: Clarified Migration README: grammar, out-of-date, organization #2141
Conversation
Signed-off-by: Kyle J. Davis <[email protected]>
sources/api/migration/README.md
Outdated
|
||
Migration code should not assume that any given keys exist, because migrations will be run on live data (where all keys will likely exist) and on pending data (where none, some, or all keys may exist). | ||
Plus, different variants of Bottlerocket may not have the same keys. | ||
|
||
The migration system could deserialize the function outputs into the incoming model types to confirm that the structure is valid; we should prototype this idea because it would add safety. | ||
The migration system deserializes the function outputs into the incoming model types to confirm that the structure is valid. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was more of a TODO comment; my sense is that it's not implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Gotcha. Inline it's not super clear that it's a todo.
What if we change the Open questions heading to Open questions and future directions and move this down to that section? That way all the maybe stuff goes into end of the document.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in 73d2b39
73d2b39
to
21dee00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except for the typo.
style nit: I prefer to prefix the first commit line with the component that's affected. In this case I'd use "docs: ...".
Co-authored-by: Ben Cressey <[email protected]> Co-authored-by: Matthew James Briggs <[email protected]> Signed-off-by: Kyle J. Davis <[email protected]>
96de0c2
to
ff312b3
Compare
Signed-off-by: Kyle J. Davis [email protected]
Issue number: N/A
Description of changes:
Testing done: Its Markdown. I used my eyeballs.
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.